Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added support for barTintColor in android ios #55

Merged
merged 8 commits into from
Oct 21, 2024

Conversation

shubhamguptadream11
Copy link
Contributor

Part of this: #11

Summary: This PR updates the tab bar background color for both iOS and Android implementations.

Changes Made:

iOS: Added the updateTabBarAppearance(with:) function to set the background color using UITabBarAppearance for iOS 15 and barTintColor for earlier versions

Android: Updated the tab bar background color to apply uniformly.

Testing:

  • Verified background color changes on both iOS and Android.
  • Added FourTabsWithBarTintColor for visual changes.

Adding snapshot of changes here:

iOS:
simulator_screenshot_6FD154D1-C2C9-4DD2-B6E9-50B27E680B62

Android:
Screenshot_20241018_142226

Copy link
Owner

@okwasniewski okwasniewski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall it looks good! Thanks for working on this, can you double check if it doesn't break any existing appearance props?

@@ -76,6 +77,9 @@ struct TabViewImpl: View {
UITabBar.appearance().scrollEdgeAppearance = configureAppearance(for: newValue ?? "")
}
}
.onAppear {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should use .onChange(of: props.barTintColor) to allow dynamic changing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the suggestion!

I initially tried using .onChange(of: props.barTintColor), but it doesn't seem to update the barTintColor as expected in my case. The color only reflects properly when using .onAppear.

To make sure the barTintColor reflects both on view load and during dynamic changes, I combined both .onAppear and .onChange(of:). This approach ensures that the UITabBar appearance is set correctly when the view first appears and updates dynamically when barTintColor changes.

Here is what I am going to change:

.onAppear {
    updateTabBarAppearance(with props.barTintColor)
}
.onChange(of: barTintColor) { newColor in
    updateTabBarAppearance(with newColor)
}

Is this okay? @okwasniewski

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If thats the case you can do a custom extension on the view, lets keep simillar logic contained:

extension View {
  @ViewBuilder
  func tabBarTintColor(enabled: Bool) -> some View {
    self
		.onAppear {
            // Do something
         }
       .onChange(of: barTintColor) { newColor in
    // Do something
      }
  }

UITabBar.appearance().standardAppearance = appearance
UITabBar.appearance().scrollEdgeAppearance = appearance
} else {
UITabBar.appearance().barTintColor = barTintColor
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't it work for newer OS versions? I didn't see deprecation in the docs.

This function should ideally only call UITabBar.appearance().barTintColor. Overriding the appearance one more time can cause issues with scrollEdgeApperance.

Copy link
Contributor Author

@shubhamguptadream11 shubhamguptadream11 Oct 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried with this approach as well: UITabBar.appearance().barTintColor on iOS 17.2 simulator.

One wierd thing happening here is when I reach end by scrolling till end barTintColor disappeared.

Attaching video for reference:

vii.mp4

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shubhamguptadream11 This looks like its related to scrollEdgeAppearance (which controls whether the tab bar should disappear when nothing is behind.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yess

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  private func updateTabBarAppearance(with barTintColor: UIColor?) {
      if (barTintColor != nil) {
          
          if #available(iOS 15.0, *) {
              let appearance = UITabBarAppearance()
              
              appearance.configureWithOpaqueBackground()
              appearance.backgroundColor = barTintColor
              
              UITabBar.appearance().standardAppearance = appearance
              UITabBar.appearance().scrollEdgeAppearance = appearance
          } else {
              UITabBar.appearance().barTintColor = barTintColor
          }
      }
  }

Why are we putting a check for iOS 15? Is barTintColor deprecated?

No, barTintColor is not deprecated. The issue I raised above requires setting scrollEdgeAppearance, which is available from iOS 15. To set this, we create a UITabBarAppearance(), making the direct use of barTintColor redundant. Instead, we use backgroundColor.

Why are we not only using barTintColor to set the tab bar's color?
This is due to the need to set scrollEdgeAppearance, as mentioned above.

Why do we check for nil for barTintColor?
We check for nil to ensure that barTintColor is applied only when the user passes it from the React Native side. This way, scrollEdgeAppearance is overridden only when barTintColor is being applied.

Let me know if you have any other suggestion.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh okay, now I get what you mean.

In this case we need to have one function which updates everything related to apperance.

private func configureAppearance(for appearanceType: String, appearance: UITabBarAppearance) -> UITabBarAppearance {
  switch appearanceType {
  case "opaque":
    appearance.configureWithOpaqueBackground()
  case "transparent":
    appearance.configureWithTransparentBackground()
  default:
    appearance.configureWithDefaultBackground()
  }
  
  return appearance
}

// Helper function to update the tab bar appearance
private func updateTabBarAppearance(props: TabViewProps) {
  if #available(iOS 15.0, *) {
    let appearance = UITabBarAppearance()
    UITabBar.appearance().scrollEdgeAppearance = configureAppearance(
      for: props.scrollEdgeAppearance ?? "",
      appearance: appearance
    )
    appearance.backgroundColor = props.barTintColor
    UITabBar.appearance().standardAppearance = appearance
  } else {
    UITabBar.appearance().barTintColor = props.barTintColor
  }

Then we can combine it together with onChange:

.onAppear {
      updateTabBarAppearance(props: props)
    }
    .onChange(of: props.barTintColor) { newValue in
      updateTabBarAppearance(props: props)
    }
    .onChange(of: props.scrollEdgeAppearance) { newValue in
      updateTabBarAppearance(props: props)
    }

This should also fix current bug with scrollEdgeAppearance because its not updated onAppear

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you rebase on top of main as I've merged your recent PR, we need to also include it here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@okwasniewski
I refactored the code to handle the onAppear and onChange logic for all TabView props in one place, simplifying the update logic. I’ve tested the changes across all three props—translucent, scrollEdgeAppearance, and barTintColor—with all their possible values to ensure everything is working as expected.

android/src/main/java/com/rcttabview/RCTTabView.kt Outdated Show resolved Hide resolved
@okwasniewski okwasniewski mentioned this pull request Oct 18, 2024
7 tasks
Copy link
Owner

@okwasniewski okwasniewski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for working on this!

@okwasniewski okwasniewski merged commit 6a2f41b into okwasniewski:main Oct 21, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants